THRIFT-5587: Add UUID support for PHP#3332
Conversation
Implement UUID as a first-class type in the PHP library and code generator. UUID (wire type 16, compact protocol type 0x0D) is represented as a string in canonical form (xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx), consistent with the Python approach. Fixed 16 bytes on wire in binary/compact protocols, JSON string in JSON protocol. Changes: - TType: add UUID = 16 constant - TProtocol: add abstract writeUuid/readUuid, UUID cases in skip/skipBinary - TBinaryProtocol: implement writeUuid/readUuid (16 raw bytes) - TCompactProtocol: add COMPACT_UUID = 0x0D, type mappings, writeUuid/readUuid - TJSONProtocol: add NAME_UUID = "uid", type mappings, writeUuid/readUuid - TSimpleJSONProtocol: add writeUuid/readUuid - TProtocolDecorator: add writeUuid/readUuid delegation - TBase: add UUID to $tmethod dispatch array - t_php_generator.cc: add TYPE_UUID in all code generation switch statements - Update test configs to use current ThriftTest.thrift (with UUID fields) - Add UUID unit tests for TBinaryProtocol and TCompactProtocol Client: php Patch: Volodymyr Panivko
|
Nice work!! Can you just ensure that the "on the wire" format is the same between your implementation and .net or c++ or one of those languages: Visual confirmation is good enough. For reference refer to this comment I made and my observations on inconsistencies in #3144 |
|
Thanks for the review! Here's the visual confirmation of wire format compatibility between PHP and C++. Test setupBoth tests were run inside an PHP test — writes UUID via $uuid = '550e8400-e29b-41d4-a716-446655440000';
$transport = new TMemoryBuffer();
$protocol = new TBinaryProtocol($transport);
$protocol->writeUuid($uuid);
$data = $transport->read(1024);
echo "PHP Binary: " . bin2hex($data) . " (" . strlen($data) . " bytes)\n";
$transport2 = new TMemoryBuffer();
$protocol2 = new TCompactProtocol($transport2);
$protocol2->writeUuid($uuid);
$data2 = $transport2->read(1024);
echo "PHP Compact: " . bin2hex($data2) . " (" . strlen($data2) . " bytes)\n";C++ test — writes the same UUID via TUuid uuid("550e8400-e29b-41d4-a716-446655440000");
auto buf = std::make_shared<TMemoryBuffer>();
TBinaryProtocol proto(buf);
proto.writeUUID(uuid);
std::string data = buf->getBufferAsString();
printf("C++ Binary: ");
for (unsigned char c : data) printf("%02x", c);
printf(" (%zu bytes)\n", data.size());
auto buf2 = std::make_shared<TMemoryBuffer>();
TCompactProtocol proto2(buf2);
proto2.writeUUID(uuid);
std::string data2 = buf2->getBufferAsString();
printf("C++ Compact: ");
for (unsigned char c : data2) printf("%02x", c);
printf(" (%zu bytes)\n", data2.size());OutputWire bytes are identical across both implementations and both protocols. UUID hex digits are written directly as 16 bytes in order — no byte swapping. |
Validate UUID format on write (all protocols) and on read (JSON protocol) using the canonical regex pattern. Throws TProtocolException on invalid input.
|
Also added UUID format validation in a follow-up commit. The This matches the approach used in the Ruby implementation ( |
I have been thinking about this since on the NodeJS side it does validation when reading values. However I don't think that validation is something that should be happen for every field for every request, I think it adds unnecessary overhead for an application that is correct (that probably does validation outside of the message exchange). Perhaps useful for debugging but not in operation. @Jens-G What is your feeling around this? Edit; see this commit I had to do on the nodets side CJCombrink@8b5576b because of the incompatibility with JAVA the node side throws away the messages since the UUID is not valid (due to the wrong byte order on the wire), this made me question 'in line' validation |
Summary
Implements UUID as a first-class type in the PHP library and code generator, as part of THRIFT-5587.
xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx) in PHP, similar to the Python implementationChanges
PHP Library:
TType::UUID = 16constantwriteUuid()/readUuid()in TBinaryProtocol, TCompactProtocol, TJSONProtocol, TSimpleJSONProtocol, TProtocolDecoratorskip()andskipBinary())COMPACT_UUID = 0x0D)$tmethodmapping for automatic read/write dispatchCode Generator (
t_php_generator.cc):TYPE_UUIDcases in all switch statements:type_to_enum,type_to_cast,type_to_phpdoc,render_const_value,generate_serialize_field(protocol + binary inline),generate_deserialize_field(protocol + binary inline)Tests:
testUuid) and client test casesTest plan
TType::UUID,readUuid,writeUuid)ThriftTest_testUuid_args.phpandThriftTest_testUuid_result.phpcontain correct UUID serialization